-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add json validation to tests #310
base: main
Are you sure you want to change the base?
add json validation to tests #310
Conversation
for more information, see https://pre-commit.ci
…nto validate_read_probe_dicts
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #310 +/- ##
==========================================
+ Coverage 89.42% 89.46% +0.04%
==========================================
Files 10 11 +1
Lines 1891 1899 +8
==========================================
+ Hits 1691 1699 +8
Misses 200 200 ☔ View full report in Codecov by Sentry. |
Here I just added a line to each of the tests. This works, but there might be a more elegant solution, for example to use parameterized tests. Feel free to refactor for style |
for more information, see https://pre-commit.ci
…nto validate_read_probe_dicts
for more information, see https://pre-commit.ci
@@ -26,10 +26,10 @@ | |||
"annotations": { | |||
"type": "object", | |||
"properties": { | |||
"name": { "type": "string" }, | |||
"model_name": { "type": "string" }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure about this name change. Both "name" and "model_name" are present as metadata, and neither required by the probe object. Should we make it required here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want the json_schema to map perfectly onto the Probe object, or might it make sense of have them hav slightly different requirements?
I added some json schema validation to the tests and found a few minor discrepancies in the json schema